Fix failed assertion in build_response for 1xx races#12866
Open
bneradt wants to merge 1 commit intoapache:masterfrom
Open
Fix failed assertion in build_response for 1xx races#12866bneradt wants to merge 1 commit intoapache:masterfrom
bneradt wants to merge 1 commit intoapache:masterfrom
Conversation
This failed assertion was seen sometimes when running the cache-tests. When a 1xx informational response (such as 103 Early Hints) is forwarded to the client, its headers are copied into client_response and sent via an asynchronous tunnel. The tunnel completion handler (tunnel_handler_100_continue) destroys client_response when the tunnel finishes. However, if the final server response arrives and build_response is called before the tunnel completes, client_response is still valid, triggering the assertion `!new_hdr->valid()` in copy_header_fields. This patch addresses this by defensively destroying outgoing_response in build_response if it is already valid before calling copy_header_fields. The header data from the 1xx has already been serialized into the tunnel buffer and is no longer needed, so this should be safe.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a race condition that causes an assertion failure in build_response when handling HTTP 1xx informational responses (such as 100 Continue or 103 Early Hints). The issue occurs when the final server response arrives before the asynchronous tunnel that forwards the 1xx response to the client completes.
Changes:
- Added a defensive check in
build_responseto destroyoutgoing_responseif it's already valid before callingcopy_header_fields - This prevents the assertion failure in
copy_header_fieldswhich expects the new header to be invalid
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This failed assertion was seen sometimes when running the cache-tests.
When a 1xx informational response (such as 103 Early Hints) is forwarded to the client, its headers are copied into client_response and sent via an asynchronous tunnel. The tunnel completion handler (tunnel_handler_100_continue) destroys client_response when the tunnel finishes. However, if the final server response arrives and build_response is called before the tunnel completes, client_response is still valid, triggering the assertion
!new_hdr->valid()in copy_header_fields.This patch addresses this by defensively destroying outgoing_response in build_response if it is already valid before calling copy_header_fields. The header data from the 1xx has already been serialized into the tunnel buffer and is no longer needed, so this should be safe.